-
-
Notifications
You must be signed in to change notification settings - Fork 229
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add the bookmarks count to tweet #286
base: main
Are you sure you want to change the base?
Conversation
Reviewer's Guide by SourceryThis pull request adds the Class diagram showing Tweet class modificationsclassDiagram
class Tweet {
+str text
+int favorite_count
+list media
+int|None bookmark_count
+__init__(client, data, user)
+__eq__(__value)
+__ne__(__value)
}
note for Tweet "Added bookmark_count attribute
fetched from public_metrics or legacy data"
Class diagram showing TweetEngagementEvent modificationsclassDiagram
class TweetEngagementEvent {
+int|None like_count
+int|None retweet_count
+str|None view_count
+str|None view_count_state
+int|None quote_count
+int|None reply_count
+int|None bookmark_count
}
note for TweetEngagementEvent "Added bookmark_count field"
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
WalkthroughThe pull request removes several example scripts and introduces a new Changes
Possibly related issues
Poem
Finishing Touches
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @ankh2054 - I've reviewed your changes - here's some feedback:
Overall Comments:
- Consider adding tests to verify bookmark count parsing from both public_metrics and legacy data structures
Here's what I looked at during the review
- 🟡 General issues: 1 issue found
- 🟢 Security: all looks good
- 🟢 Testing: all looks good
- 🟢 Complexity: all looks good
- 🟢 Documentation: all looks good
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
🧹 Nitpick comments (2)
example_project/test_twikit.py (1)
4-16
: Add docstring to explain script purpose and usage.The script would benefit from a docstring explaining its purpose, required environment variables, and usage instructions.
async def main(): + """ + Example script demonstrating tweet bookmark count retrieval. + + Environment variables: + TWIKIT_COOKIES_PATH: Path to the cookies.json file + TWEET_ID: ID of the tweet to fetch + """ client = Client()setup.py (1)
Line range hint
15-21
: Consider using minimum version requirements.Instead of pinning exact versions, consider specifying minimum versions to allow for compatible updates and security patches.
install_requires=[ - 'httpx[socks]', - 'filetype', - 'beautifulsoup4', - 'pyotp', - 'lxml' + 'httpx[socks]>=0.24.0', + 'filetype>=1.2.0', + 'beautifulsoup4>=4.11.0', + 'pyotp>=2.8.0', + 'lxml>=4.9.0' ],
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (13)
.gitignore
(1 hunks)example_project/test_twikit.py
(1 hunks)examples/delete_all_tweets.py
(0 hunks)examples/dm_auto_reply.py
(0 hunks)examples/download_tweet_media.py
(0 hunks)examples/example.py
(0 hunks)examples/guest.py
(0 hunks)examples/listen_for_new_tweets.py
(0 hunks)setup.py
(3 hunks)twikit/client/__init__.py
(1 hunks)twikit/guest/tweet.py
(2 hunks)twikit/streaming.py
(2 hunks)twikit/tweet.py
(2 hunks)
💤 Files with no reviewable changes (6)
- examples/guest.py
- examples/download_tweet_media.py
- examples/example.py
- examples/delete_all_tweets.py
- examples/dm_auto_reply.py
- examples/listen_for_new_tweets.py
✅ Files skipped from review due to trivial changes (1)
- .gitignore
🚧 Files skipped from review as they are similar to previous changes (3)
- twikit/tweet.py
- twikit/streaming.py
- twikit/guest/tweet.py
🔇 Additional comments (2)
twikit/client/__init__.py (1)
1-4
: LGTM! Well-structured module initialization.The imports and
__all__
declaration follow Python best practices for module organization and explicit public interface definition.setup.py (1)
13-13
: LGTM! Good use of find_packages().Using
find_packages()
is the recommended approach for automatic package discovery.
client = Client() | ||
|
||
# Load your saved cookies if you have them | ||
client.load_cookies('path/to/cookies.json') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Use environment variables or command-line arguments for the cookies path.
Hardcoded paths reduce flexibility and portability. Consider using environment variables or command-line arguments.
- client.load_cookies('path/to/cookies.json')
+ cookies_path = os.getenv('TWIKIT_COOKIES_PATH', 'cookies.json')
+ client.load_cookies(cookies_path)
Committable suggestion skipped: line range outside the PR's diff.
tweet = await client.get_tweet_by_id('1519480761749016577') | ||
print(f"Tweet bookmark count: {tweet.bookmark_count}") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Add error handling and make tweet ID configurable.
The script needs proper error handling for API calls and should make the tweet ID configurable.
- tweet = await client.get_tweet_by_id('1519480761749016577')
- print(f"Tweet bookmark count: {tweet.bookmark_count}")
+ tweet_id = os.getenv('TWEET_ID', '1519480761749016577')
+ try:
+ tweet = await client.get_tweet_by_id(tweet_id)
+ print(f"Tweet bookmark count: {tweet.bookmark_count}")
+ except Exception as e:
+ print(f"Error fetching tweet: {e}")
Committable suggestion skipped: line range outside the PR's diff.
@@ -24,5 +25,5 @@ | |||
long_description_content_type='text/markdown', | |||
license='MIT', | |||
url='https://github.com/d60/twikit', | |||
package_data={'twikit': ['py.typed']} | |||
include_package_data=True |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Codebase verification
Remove include_package_data=True
as it's not needed
The codebase doesn't contain any non-Python package data files that need to be included in the distribution. The existing non-Python files are development-related (documentation, requirements, linter config) and don't need to be packaged. You can safely remove this setting from setup.py.
🔗 Analysis chain
Verify MANIFEST.in file exists.
The include_package_data=True
setting requires a MANIFEST.in file to specify which non-Python files should be included in the distribution.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check if MANIFEST.in exists and contains necessary include statements
if [ -f "MANIFEST.in" ]; then
echo "MANIFEST.in contents:"
cat MANIFEST.in
else
echo "Warning: MANIFEST.in file not found"
fi
Length of output: 105
Script:
#!/bin/bash
# Search for non-Python files that might need to be included in distribution
fd --type f \
--exclude '*.py' \
--exclude '*.pyc' \
--exclude '__pycache__' \
--exclude '.git' \
--exclude '.github' \
--exclude 'tests' \
--exclude '*.md' \
--exclude 'LICENSE' \
--exclude '.gitignore' \
--exclude 'setup.py' \
--exclude 'setup.cfg' \
--exclude 'pyproject.toml' \
--exclude '*.ini'
Length of output: 365
As per twitter API docs - https://developer.x.com/en/docs/x-api/tweets/lookup/api-reference/get-tweets-id
Summary by Sourcery
New Features:
bookmark_count
field to theTweet
object, allowing access to the number of times a tweet has been bookmarked.Summary by CodeRabbit
New Features
Bug Fixes
__ne__
method implementation for consistent comparison behavior.Documentation
Chores
.gitignore
to exclude unnecessary files and directories.setup.py
for better package discovery.